Skip to content

Conversation

samueltardieu
Copy link
Member

.map_or(false, f) on a Option or a Result was not detected by manual_is_variant_and.

This change is small (first commit), but induces a lot of (rightful) fixes (second commit) in Clippy sources.

changelog: [manual_is_variant_and]: add detection of .map_or(false, f) pattern

@rustbot
Copy link
Collaborator

rustbot commented Oct 6, 2024

r? @Manishearth

rustbot has assigned @Manishearth.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 6, 2024
@y21
Copy link
Member

y21 commented Oct 6, 2024

FWIW, this is also being implemented in #11796, though as a new lint and not part of manual_is_variant_and since the PR predates it

@samueltardieu
Copy link
Member Author

FWIW, this is also being implemented in #11796, though as a new lint and not part of manual_is_variant_and since the PR predates it

Oh. So what do we do? Although I think this check rather belongs to manual_is_variant_of and should be machine applicable, I don't mind dropping it if @Jacherr prefer to finalize their PR instead.

@samueltardieu
Copy link
Member Author

Moreover, @Jacherr PR could be retargeted to catch instances of .is_some_and(|x| x == V) (or .is_ok_and(|x| x == V)) and propose rewriting them as == Some(V) or == Ok(V). It could be applied in some cases after the current PR, and will catch more cases of code already using .is_some_and() and .is_ok_and().

@Jacherr
Copy link
Contributor

Jacherr commented Oct 6, 2024

The existing PR is pretty far through the review process and as far as I know just needs a few more changes before being ready for merging. I just need time to get round to it!

As is, I’m not sure there’s much benefit complicating the situation even further with that PR because it’s been open for almost a year now (oops..) As for these improvements, they can probably be retroactively applied after the fact (either by myself or you).

@samueltardieu
Copy link
Member Author

Fine with me, I'll keep this PR in my own version for the time being so that I can benefit from the lint (I detected some usages of .map_or(false, …) in some of my code and got it fixed using it) until your PR is merged, and I'll close it when your is in.

@Manishearth Manishearth removed their assignment Oct 7, 2024
@Manishearth
Copy link
Member

On vacation for a week. Feel free to retarget review with r? clippy

@samueltardieu samueltardieu deleted the manual-is-variant-and branch January 6, 2025 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants